Skip to content

Conversation

@LiteSun
Copy link
Contributor

@LiteSun LiteSun commented Oct 30, 2025

Description

  1. Fix type errors across all projects
    sdk differ converter-openapi backend-api7 backend-apisix-standalone backend-apisix cli
  2. Add a CI check for type checking
image

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible

@LiteSun LiteSun requested a review from Copilot November 4, 2025 07:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR performs a major dependency downgrade, changing Vitest from version 3.2.4 to 3.0.0, alongside TypeScript type safety improvements and code quality enhancements across multiple backend implementations (APISIX, API7) and the SDK. The changes include adding explicit type assertions, fixing type compatibility issues, and improving null safety handling throughout the codebase.

  • Downgraded vitest from 3.2.4 to 3.0.0 with corresponding dependency adjustments
  • Added type assertions and explicit casting to resolve TypeScript strict mode violations
  • Refactored schema definitions to better support plugin configuration types

Reviewed Changes

Copilot reviewed 32 out of 34 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
pnpm-lock.yaml Downgraded vitest and related dependencies from 3.2.4 to 3.0.0, added package dependencies to apps/cli and libs/sdk
libs/sdk/src/core/schema.ts Changed pluginsSchema from record type to single plugin schema, added UpstreamHealthCheck type export, extended ConfigurationSchema with new optional fields
libs/sdk/src/backend/index.ts Made httpAgent and httpsAgent optional in BackendOptions
libs/backend-apisix/src/transformer.ts Added extensive type assertions and null-safety checks throughout transformation methods
libs/backend-apisix/src/typing.ts Changed GlobalRule and PluginMetadata from record types to direct types
libs/backend-api7/src/transformer.ts Improved type safety with explicit return types and type assertions
libs/backend-api7/src/index.ts Added non-null assertions and improved type guards in defaultValue method
apps/cli/src/utils/listr.ts Replaced logical AND shortcuts with explicit if statements for better readability
apps/cli/package.json Moved dependencies from devDependencies to dependencies
.github/workflows/lint.yaml Added new GitHub Actions workflow for lint checking
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

LiteSun and others added 4 commits November 4, 2025 18:49
…in permissions

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@LiteSun LiteSun changed the title fix(apps): lint chore(types): add type check Nov 4, 2025
@LiteSun LiteSun marked this pull request as ready for review November 5, 2025 00:29
@LiteSun LiteSun requested a review from bzp2010 as a code owner November 5, 2025 00:29
@LiteSun LiteSun added test/api7 Trigger the API7 test on the PR test/apisix-standalone Trigger the APISIX standalone test on the PR labels Nov 5, 2025
@LiteSun LiteSun marked this pull request as draft November 5, 2025 01:35
@LiteSun LiteSun marked this pull request as ready for review November 5, 2025 03:35
Comment on lines +379 to +382
routes: z.array(routeSchema).optional(),
stream_routes: z.array(streamRouteSchema).optional(),
consumer_credentials: z.array(consumerCredentialSchema).optional(),
upstreams: z.array(upstreamSchema({ id: idSchema.optional() })).optional(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These elements do not exist in the ADC schema and users are not permitted to configure these fields here. Do not modify the core schema.
If your type system requires these fields, create another type alias.

const AnotherConfiguration = z.infer<typeof ConfigurationSchema> & {xx: xx, yy: yy};


private listGlobalRules() {
return this._list<typing.ListResponse<typing.GlobalRule>>(
return this._list<typing.ListResponse<Record<string, typing.GlobalRule>>>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

ADC_NAME: consumerGroup.name,
},
name: undefined,
consumers: undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to allow recursiveOmitUndefined to remove them; please leave it as is.

First, the id, name, consumers expanded from consumerGroup will be overwritten with undefined, then subsequently removed by recursiveOmitUndefined—that's how they function.
Additionally, this modification is useless, as ADC does not support Consumer Group; thus, they are now essentially placeholders.

"esnext"
]
],
"skipLibCheck": true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test/api7 Trigger the API7 test on the PR test/apisix-standalone Trigger the APISIX standalone test on the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants